Lazy sync of Thread::m_LastThrownObjectHandle from ExInfo::m_exception by max-charlamb · Pull Request #127649 · dotnet/runtime

max-charlamb · 2026-05-01T14:51:14Z

Note

This PR was authored with the assistance of GitHub Copilot.

Summary

Follow-up to #127300. Stop eagerly synchronizing Thread::m_LastThrownObjectHandle with ExInfo::m_exception during active exception dispatch. Instead, set LTO lazily when the ExInfo is destroyed in PopExInfos.

After #127300 made m_exception the GC-tracked exception object during dispatch, the eager LTO sync became redundant cost — every throw and dispatch transition was creating/updating a GCHandle that nothing on the dispatch path actually consumes (consumers read m_exception directly, or only need LTO once dispatch unwinds).

Key Changes

Lazy LTO update: ExInfo::~ExInfo (via PopExInfos) writes m_exception into m_LastThrownObjectHandle exactly once, when the exception leaves the dispatch chain. m_exception remains the sole source of truth during dispatch.
Remove SafeSetThrowables — the dual-write helper is no longer needed.
Remove SafeUpdateLastThrownObject — eager sync sites are gone.
Remove SyncManagedExceptionState and the InternalUnhandledExceptionFilter re-sync block.

Cleanup of dead exception-handling helpers

While reviewing this PR, @jkotas pointed out additional dead code that the lazy-LTO change exposed, plus other latent dead code in the area:

Merge SafeSetLastThrownObject into SetLastThrownObject — single public method, NOTHROW contract. The EX_TRY/EX_CATCH now wraps only the throwing CreateHandle call; observable behavior on the OOM fallback path is unchanged.
Delete Thread::SetLastThrownObjectHandle — zero callers post-PR.
Delete SetManagedUnhandledExceptionBit forward declaration — no definition existed anywhere in the codebase.
Drop the always-NULL pThrowableIn parameter of NotifyAppDomainsOfUnhandledException.
Replace UpdateCurrentThrowable (whose name was misleading: it never updated anything, only returned a boolean) with an inline check at the single call site using the GC-mode-agnostic IsThrowableNull / IsLastThrownObjectNull helpers.
Delete CEEInfo::HandleException — unreachable since 2016 (4d9f4b8 "Remove SEH interactions between the JIT and the EE" replaced the old ICorJitInfo::FilterException/HandleException pair with runWithErrorTrap). The function is private, non-virtual, not part of the ICorJitInfo interface, and has zero callers in coreclr, the JIT, the AOT thunks, or SuperPMI. Removing it also retires a long-stale comment about "sync between the LTO and the exception tracker" that pre-dates the ExInfo redesign.

What stays unchanged

m_LastThrownObjectHandle itself remains an OBJECTHANDLE — required by the ICorDebug managed debugging protocol, which reads it cross-process via BuildFromGCHandle.
GetCurrentException / GetThreadException still prefer the live ExInfo::m_exception (via pseudo-handle) and only fall back to LTO when no ExInfo is active — same precedence as Remove ExInfo::m_hThrowable - use direct pointer for exception objects #127300.
Preallocated-exception handling (SO, OOM) is preserved.
The m_ltoIsUnhandled flag is kept — it is still set TRUE from EEPolicy::HandleFatalError / HandleFatalStackOverflow and consumed by the managed debugger via DAC and cDAC.

Testing

Ran internal diagnostics tests with no regressions

…eagerly Stop eagerly synchronizing m_LastThrownObjectHandle with ExInfo::m_exception during active exception dispatch. Instead, set LTO lazily when the ExInfo is destroyed in PopExInfos. This simplifies the code and makes m_exception the sole source of truth during dispatch. Removed: SafeSetThrowables, SafeUpdateLastThrownObject, SyncManagedExceptionState, and the InternalUnhandledExceptionFilter re-sync block. Made SetLastThrownObject private; all external callers now use SafeSetLastThrownObject which handles OOM gracefully. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

max-charlamb · 2026-05-01T14:53:17Z

@EgorBot -intel -amd

using BenchmarkDotNet.Attributes;
using System.Runtime.CompilerServices;

[MemoryDiagnoser]
public class ExceptionBench
{
    [Benchmark]
    public int SimpleThrowCatch()
    {
        try { throw new InvalidOperationException("x"); }
        catch (Exception e) { return e.Message.Length; }
    }

    [Benchmark]
    public int Rethrow()
    {
        try
        {
            try { throw new InvalidOperationException("x"); }
            catch { throw; }
        }
        catch (Exception e) { return e.Message.Length; }
    }

    [Benchmark]
    public int CatchAndThrowNew()
    {
        try
        {
            try { throw new InvalidOperationException("inner"); }
            catch { throw new ApplicationException("outer"); }
        }
        catch (Exception e) { return e.Message.Length; }
    }

    [Benchmark]
    public int NestedThrowCatch()
    {
        int r = 0;
        try { Outer(); }
        catch (Exception e) { r = e.Message.Length; }
        return r;
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Outer()
    {
        try { Inner(); }
        catch (InvalidOperationException) { throw new ApplicationException("rewrapped"); }
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Inner() => throw new InvalidOperationException("x");

    [Benchmark]
    public int DeepThrowCatch()
    {
        try { Recurse(20); return 0; }
        catch (Exception e) { return e.Message.Length; }
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    static void Recurse(int n)
    {
        if (n == 0) throw new InvalidOperationException("deep");
        Recurse(n - 1);
    }
}

The interesting cases here:

Bench	What it stresses
`SimpleThrowCatch`	One `ExInfo` lifetime — was 1 eager LTO write + 1 reset; now 1 write at `PopExInfos`
`Rethrow`	`SafeUpdateLastThrownObject` on rethrow — entirely removed
`CatchAndThrowNew`	New throw inside a catch — an extra eager LTO sync per dispatch transition
`NestedThrowCatch`	Multi-frame transition: previously two eager writes, now one lazy write
`DeepThrowCatch`	Pure unwind cost (control: shouldn't change much)

Expecting the rethrow/catch-and-throw-new paths to show the largest delta since those are the dispatch transitions where SafeUpdateLastThrownObject / SafeSetThrowables used to run.

Copilot

Pull request overview

This PR refactors CoreCLR exception state tracking to avoid eagerly synchronizing Thread::m_LastThrownObjectHandle (LTO) with ExInfo::m_exception during active exception dispatch, instead updating LTO lazily when ExInfo entries are popped. The intent is to reduce per-throw overhead after ExInfo::m_exception became the primary GC-tracked exception object during dispatch (per #127300).

Changes:

Update LTO lazily in ExInfo::PopExInfos from ExInfo::m_exception as each tracker is popped.
Remove eager-sync helpers/paths (SafeSetThrowables, SafeUpdateLastThrownObject, SyncManagedExceptionState) and switch call sites to SafeSetLastThrownObject.
Restrict direct Thread::SetLastThrownObject usage by making it private and routing external callers through SafeSetLastThrownObject (now optionally marks unhandled).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/coreclr/vm/threads.h	Updates LTO documentation, removes eager-sync APIs, makes `SetLastThrownObject` private, extends `SafeSetLastThrownObject` signature.
src/coreclr/vm/threads.cpp	Removes `SafeSetThrowables`/`SafeUpdateLastThrownObject` implementations; updates thread teardown to clear LTO via `SafeSetLastThrownObject`.
src/coreclr/vm/exinfo.cpp	Adds lazy LTO update when popping `ExInfo` entries.
src/coreclr/vm/exceptionhandling.cpp	Removes eager LTO sync at first-chance notification and removes `SyncManagedExceptionState` call after catch funclets.
src/coreclr/vm/excep.cpp	Removes unhandled-exception filter resync of LTO against the active tracker.
src/coreclr/vm/eepolicy.cpp	Routes fatal error/SO paths through `SafeSetLastThrownObject(..., TRUE)` instead of `SetLastThrownObject` / `SafeSetThrowables`.
src/coreclr/vm/comutilnative.cpp	Uses `SafeSetLastThrownObject` in FailFast path.

The reviewer noted that PopExInfos now reads m_exception (an OBJECTREF) and calls Thread::SafeSetLastThrownObject, which requires MODE_COOPERATIVE for non-NULL throwables, without an explicit GC mode contract. All six callers were already cooperative: - exceptionhandling.cpp:601 - explicit GCX_COOP() three lines above - CleanUpForSecondPass callers (lines 2084/2139/2148) - the asm stub puts the thread in COOP before entering, and the SO path uses GCX_COOP_NO_DTOR - CallCatchFunclet (line 3185) - has MODE_COOPERATIVE CONTRACTL itself - ResumeAtInterceptionLocation (line 3300) - catch-dispatch path also calls PopExplicitFrames, which already requires cooperative mode Add the contract explicitly so that future callers cannot violate it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@jkotas

Cleans up dead code identified by @jkotas in PR dotnet#127649 review: - Delete Thread::SetLastThrownObjectHandle: zero callers post-PR. - Delete SetManagedUnhandledExceptionBit forward declaration: had no definition anywhere in the codebase. - Remove the always-NULL pThrowableIn parameter from NotifyAppDomainsOfUnhandledException. - Replace UpdateCurrentThrowable (whose name was misleading: it never updated anything, only returned a boolean) with an inline check at the single call site using the GC-mode-agnostic IsThrowableNull / IsLastThrownObjectNull helpers, removing the need for a GCX_COOP inside the surrounding PAL_TRY block. - Merge SafeSetLastThrownObject into SetLastThrownObject: a single public method whose contract reflects the safe NOTHROW behavior. The EX_TRY/EX_CATCH wraps only the throwing CreateHandle call; observable behavior on the OOM fallback path is unchanged. - Drop a stale `similar to UpdateCurrentThrowable()` comment in eedbginterfaceimpl.cpp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Two changes responding to PR review feedback: 1. Remove dead CEEInfo::HandleException. The function has been unreachable since 2016 (commit 4d9f4b8 `Remove SEH interactions between the JIT and the EE'') which replaced the old ICorJitInfo::FilterException/HandleException pair with runWithErrorTrap. The function is private, non-virtual, not part of the ICorJitInfo interface, and has zero callers in coreclr, the JIT, the AOT thunks, or SuperPMI (the SuperPMI Packet_HandleException slot is commented out). Removing it also retires the long-stale comment about `sync between the LTO and the exception tracker'' that pre-dates the ExInfo redesign and the lazy-LTO model from dotnet#127300/dotnet#127649. 2. Reorder declarations in threads.h so SetLastThrownObject precedes SetSOForLastThrownObject, matching the order of the definitions in threads.cpp. Also update an unrelated stale comment in ExInfo::PopExInfos: the `unmanaged thread'' rationale is incorrect because both UMThunkUnwindFrameChainHandler and CallDescrWorkerUnwindFrameChainHandler short-circuit unmanaged threads before reaching PopExInfos, and the function carries a MODE_COOPERATIVE contract. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jkotas · 2026-05-01T20:54:34Z

        {
            LOG((LF_EH, LL_INFO100, "InternalUnhandledExceptionFilter_Worker: Not collecting bucket information as thread object does not exist\n"));
        }



Suggested change

jkotas · 2026-05-01T21:02:07Z

@@ -1470,8 +1470,6 @@ class Thread
    }




Suggested change

jkotas · 2026-05-01T21:02:43Z

+    // LTO may be stale during active dispatch. Readers that need the current
+    // exception should call GetThrowable() first and fall back to LastThrownObject()
+    // only when GetThrowable() returns NULL.
    OBJECTHANDLE m_LastThrownObjectHandle;      // Unsafe to use directly.  Use accessors instead.


Suggested change

OBJECTHANDLE m_LastThrownObjectHandle; // Unsafe to use directly. Use accessors instead.

OBJECTHANDLE m_LastThrownObjectHandle;

jkotas · 2026-05-01T21:34:48Z

        }
 #endif // DEBUGGING_SUPPORTED

+        // Set LTO from the exception being destroyed so that post-ExInfo consumers


We should do this only when it is actually going to be needed. For example, in a simple try { throw new Exception(); } catch { }, PopExInfos is going to be called from CallCatchFunclets, we create the LTO handle and the very next line in CallCatchFunclets is going to delete the LTO handle.

jkotas · 2026-05-01T21:48:55Z

+        pThread->SetLastThrownObject(NULL);
    }

-    // Sync managed exception state, for the managed thread, based upon any active exception tracker


Is this going to cause a behavior change in things like the windbg !pe command ?

Before this change, !pe issued while the thread is executing a catch block is going to print the exception that's being caught. After this change, I think it is going to print nothing.

(It would be nice if we can fix this without short-lived GCHandle allocation.)

Copilot AI review requested due to automatic review settings May 1, 2026 14:51

Copilot started reviewing on behalf of max-charlamb May 1, 2026 14:51 View session

github-actions Bot added the area-ExceptionHandling-coreclr label May 1, 2026

dotnet-policy-service Bot assigned max-charlamb May 1, 2026

EgorBot mentioned this pull request May 1, 2026

Benchmarks for dotnet/runtime#127649 (for @max-charlamb) EgorBot/Benchmarks#174

Open

max-charlamb marked this pull request as ready for review May 1, 2026 14:57

max-charlamb requested review from janvorli and jkotas May 1, 2026 14:57

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/coreclr/vm/exinfo.cpp

jkotas reviewed May 1, 2026

View reviewed changes

Comment thread src/coreclr/vm/threads.cpp Outdated

jkotas reviewed May 1, 2026

View reviewed changes

Comment thread src/coreclr/vm/threads.cpp Outdated

jkotas reviewed May 1, 2026

View reviewed changes

Comment thread src/coreclr/vm/excep.cpp Outdated

jkotas reviewed May 1, 2026

View reviewed changes

Comment thread src/coreclr/vm/excep.cpp Outdated

Copilot AI review requested due to automatic review settings May 1, 2026 20:13

Copilot started reviewing on behalf of max-charlamb May 1, 2026 20:13 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

Comment thread src/coreclr/vm/exinfo.cpp

Comment thread src/coreclr/vm/threads.h

Comment thread src/coreclr/vm/jitinterface.cpp Outdated

max-charlamb force-pushed the dev/max-charlamb/lazy-lto-sync-v2 branch from 09de907 to 70188bb Compare May 1, 2026 21:20

max-charlamb requested a review from jkotas May 1, 2026 21:23

jkotas reviewed May 1, 2026

View reviewed changes

	OBJECTHANDLE m_LastThrownObjectHandle; // Unsafe to use directly. Use accessors instead.
	OBJECTHANDLE m_LastThrownObjectHandle;

Conversation

max-charlamb commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Key Changes

Cleanup of dead exception-handling helpers

What stays unchanged

Testing

Uh oh!

max-charlamb commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas May 1, 2026

Choose a reason for hiding this comment

Uh oh!

jkotas May 1, 2026

Choose a reason for hiding this comment

Uh oh!

jkotas May 1, 2026

Choose a reason for hiding this comment

Uh oh!

jkotas May 1, 2026

Choose a reason for hiding this comment

Uh oh!

jkotas May 1, 2026

Choose a reason for hiding this comment

Uh oh!

jkotas May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

max-charlamb commented May 1, 2026 •

edited

Loading

max-charlamb commented May 1, 2026 •

edited

Loading